Add __all__ to hand-written modules; simplify __init__.py template#25
Conversation
pdoc uses __all__ to gate which submodules it discovers. Without the module names in __all__, pdoc only rendered the flat symbol list and skipped the submodule pages with their module-level docstrings.
Delete the 69-line custom template override. Instead, add __all__ to each hand-written module (Python best practice) and use a 37-line post-hook script that reads those declarations via AST to extend the generated __init__.py. The only hand-maintained list is the 7 module names; all exported symbols are derived from the code itself.
Use star imports from modules that define __all__ (Python best practice) and compose __init__.__all__ dynamically from submodule __all__ lists. Template shrinks from 69 lines to 24 with no external script. All exported symbols are derived from the code itself.
Module names listed once; Jinja2 generates the imports and __all__ composition. Template shrinks from 24 to 13 lines.
Ruff cannot statically resolve the dynamic __all__ and flags the explicit imports as unused. Add F401/F403 suppression comments.
Replace set-union with splat unpacking in a single set literal. Fewer Jinja brace escapes and consistent use of the for-loop pattern.
Collapse __all__ block to single line (ruff reformats the output) and remove unnecessary blank line.
Consistent whitespace control on all tags, readable __all__ block with one entry per line, blank line separating imports from __all__.
- Rename mods -> modules (descriptive names, not abbreviations) - Remove all -%} dash modifiers (trim_blocks and lstrip_blocks are enabled in the Environment, making manual trimming unnecessary)
splch
changed the title
guenp
left a comment
guenp
left a comment
There was a problem hiding this comment.
🤖 Codex Review
Automated review by OpenAI Codex (gpt-5.4), triggered by @guen-p.
Summary
This PR adds explicit __all__ declarations to all 7 hand-written modules and rewrites the package_init.py.jinja template to use star imports composed from those __all__ lists. The template shrinks from ~69 lines to 17, and adding a new public symbol only requires updating the source module's __all__ — no template changes needed.
Observations
- Looks correct. Each module's
__all__matches the symbols that were previously imported explicitly in__init__.py. The composed__all__in the generated__init__.pycovers all 39 symbols (verified by the test plan). noqacomments are appropriate.F403(star imports) andF401(unused imports) are the right suppressions here since the star imports are controlled by__all__.clientandtypesstay explicit — presumably because they're auto-generated and don't declare__all__. This is the right call.
Minor suggestions
__version__inionq_client.__all__: Re-exporting a dunder via__all__is slightly unusual. Consider whether__version__should live in its own top-level assignment (e.g. in__init__.pydirectly) rather than being star-imported fromionq_client. Not a blocker — just a style preference.__all__placement: The__all__declarations are placed above the imports (e.g. ingates.py,__all__comes beforeimport cmath). PEP 8 / isort convention typically puts imports first. Consider moving__all__to just after the imports and module docstring for consistency. Again, not a blocker.
Verdict
Looks good overall — clean simplification with solid test coverage. The two notes above are style nits, not correctness issues. 👍
guenp
left a comment
guenp
left a comment
There was a problem hiding this comment.
🤖 Codex Review
This is an automated review by Codex (OpenAI).
Summary: this PR adds explicit __all__ lists to the hand-written modules and simplifies __init__.py generation by deriving package exports from those module-level __all__ lists, while also exposing submodule names for pdoc discovery.
- I did not spot any correctness issues in the export refactor during this light review.
- Minor suggestion: adding the module objects to package
__all__changesfrom ionq_core import *semantics slightly, so it may be worth calling that out explicitly as an intentional API/docs tradeoff. - If the
39 symbols importable from ionq_corecheck is not already enforced in the automated test suite, a small regression test would help keep the package export surface stable as modules evolve.
Overall assessment: looks good overall for a light review.
guenp
left a comment
guenp
left a comment
There was a problem hiding this comment.
lgtm, I did human + ai review, approved with codex suggestions above!
PEP 8's "Module Level Dunder Names" rule places __all__ after the docstring but before any non-__future__ imports. The other hand-written modules already follow this; extensions.py was the outlier.
|
Thanks for the thorough review! Addressed one and as for the others:
|
2 participants
Summary
__all__to each hand-written module (Python best practice for explicit public APIs)package_init.py.jinjato use star imports and dynamic__all__composition from submodule__all__lists__all__so pdoc discovers submodule pages with their module-level docstringsHow it works
Each hand-written module declares
__all__(e.g.,__all__ = ["SessionManager"]). The template star-imports from each module and composes the package__all__by unpacking*module.__all__. When a symbol is added to any module's__all__, it automatically appears in the package exports with no template changes needed.Test plan
uv run pytest tests/- 219 passed, 100% coverageuv run ruff check- cleanionq_coreuv run pdocgenerates 7 submodule pages with module-level docstrings